Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add faucet enabling testing #270

Merged
merged 29 commits into from
Mar 29, 2024
Merged

Add faucet enabling testing #270

merged 29 commits into from
Mar 29, 2024

Conversation

phklive
Copy link
Contributor

@phklive phklive commented Mar 11, 2024

The Miden node generates 2 accounts by default at genesis:

  • a Faucet account
  • a regular account

We need to make the faucet available and usable on node spin-up, this would enable request of funds and testing of tx's from users of the network.

In this PR I propose to add a new crate called faucet to the node workspace and use actix-web to create a web-server that will serve html, css and js static files.

The faucet receives requests of funds through an endpoint where users on the frontend pass their AccountId. The server responds with the note generated by the tx. The end user is then able to import this newly received note into their client and consume it locally.

closes: #215

@phklive phklive requested a review from hackaugusto March 14, 2024 12:16
@igamigo
Copy link
Collaborator

igamigo commented Mar 14, 2024

One thing to keep in mind is that this PR will be merged soon (and the Client will be released to crates.io either today or tomorrow) so that might imply some minor changes here.

@phklive phklive marked this pull request as ready for review March 25, 2024 11:50
@phklive phklive requested review from Dominik1999 and bobbinth March 25, 2024 11:52
@Dominik1999 Dominik1999 removed the request for review from hackaugusto March 25, 2024 17:36
@Dominik1999 Dominik1999 requested review from igamigo and bobbinth and removed request for Dominik1999 and bobbinth March 25, 2024 17:36
@Dominik1999
Copy link
Contributor

Let merge :)

@igamigo can you review this today or tomorrow?

@igamigo
Copy link
Collaborator

igamigo commented Mar 26, 2024

Let merge :)

@igamigo can you review this today or tomorrow?

Will be checking it out in a few hours!

Copy link
Collaborator

@igamigo igamigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left some non-blocking considerations.
A couple more general points:

  • For the website, it seems that the button should post to the server and so you would get some feedback from the browser in terms of the request. The deployed website did not show any feedback and this would be confusing in terms of waiting for the response
  • The deployed website is still up, should it be taken offline for now?

faucet/src/main.rs Outdated Show resolved Hide resolved
faucet/src/main.rs Outdated Show resolved Hide resolved
faucet/src/utils.rs Outdated Show resolved Hide resolved
faucet/src/handlers.rs Outdated Show resolved Hide resolved
faucet/Cargo.toml Outdated Show resolved Hide resolved
faucet/miden-faucet.toml Outdated Show resolved Hide resolved
faucet/src/config.rs Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
@phklive phklive merged commit d8edb09 into next Mar 29, 2024
5 checks passed
@phklive phklive deleted the phklive-pol-faucet branch March 29, 2024 09:55
@bobbinth
Copy link
Contributor

@phklive - could you add a short README to the faucet crate? Just something very simple which explains its purpose and how to use it.

Comment on lines +1 to +4
[package]
name = "miden-node-faucet"
version = "0.1.0"
edition = "2021"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do this the same way as we do it in all other crates (e.g., see here).

version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this can probably be deleted.

Comment on lines +12 to +27
[dependencies]
actix-web = "4"
actix-files = "0.6.5"
actix-cors = "0.7.0"
derive_more = "0.99.17"
figment = { version = "0.10", features = ["toml", "env"] }
miden-lib = { workspace = true }
miden-client = { version = "0.1.0", features = ["concurrent"] }
miden-node-proto = { path = "../proto", version = "0.2" }
miden-node-utils = { path = "../utils", version = "0.2" }
miden-objects = { workspace = true }
serde = { version = "1.0", features = ["derive"] }
clap = { version = "4.5.1", features = ["derive"] }
async-mutex = "1.4.0"
tracing = { workspace = true }
tracing-subscriber = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's alphabetize the dependencies.

bobbinth pushed a commit that referenced this pull request Apr 12, 2024
* ci: turn doc warnings into errors (#259)

* Boilerplate done

* pushed fix for miden-client

* Fix formatting

* Pulled after client fix

* Fixed typos + added build_client fn

* Added configs + figment

* Fix client implementation in faucet + use released client

* updated cargo

* Fixed Miden node when importing Miden client, db errors

* Format files

* Added proper support of configuration file + logging

* Improved config file handling

* Removed superfluous file

* First pass at improvements

* Change naming of note

* Fixed html + added metadata endpoint

* Want to implement Display for NoteId

* Upgraded js to use async + NoteId does not impl Display in main

* cargo updated

---------

Co-authored-by: Augusto Hack <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants